[Extensions] Add W3CTraceState - #5115
Open
PetarJerinic wants to merge 7 commits into
Open
Conversation
github-actions
Bot
added
the
comp:extensions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5115 +/- ##
==========================================
- Coverage 78.27% 78.25% -0.03%
==========================================
Files 494 485 -9
Lines 20763 20621 -142
==========================================
- Hits 16253 16137 -116
+ Misses 4510 4484 -26
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PetarJerinic
marked this pull request as ready for review
August 23, 2026 22:45
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-27 18:54 UTC Review the latest changes. Status above doesn't look right?
|
Comment on lines
+176
to
+194
| Parsing always succeeds, a mutating operation that changes something returns a | ||
| new instance, and neither throws: an operation that changes nothing, such as one | ||
| naming an invalid key or value, hands back the receiver itself. Members this | ||
| instance did not generate are preserved verbatim, including malformed ones, so | ||
| that an arbitrary sequence of mutations does not erode another vendor's entries. | ||
|
|
||
| `TryParse` is available where a caller wants to know that a header was unusable. | ||
| It returns `false` when members were retained and not one of them matched the | ||
| `list-member` grammar, and `true` otherwise, including for an absent or empty | ||
| header and for a header mixing valid pairs with text kept verbatim. The state is | ||
| populated the same way whichever value is returned, so `false` never yields less | ||
| than `true` would. | ||
|
|
||
| The value reports on the members retained, not on the header as it arrived. At | ||
| most 32 members are kept either way, so a header of 32 unusable members followed | ||
| by a well-formed pair reports `false`, because the pair sits past the limit and | ||
| was never taken on. Discarding well-formed members to stay inside that limit is | ||
| not itself a failure: a header of 40 valid pairs keeps the first 32 and reports | ||
| `true`. |
Member
There was a problem hiding this comment.
This feels a bit overly descriptive to me.
Comment on lines
+209
to
+215
| The modified key is placed first and every other member keeps its relative | ||
| position, as the specification requires. At most 32 members are kept, which is | ||
| all the header grammar allows, and the right-most ones are dropped as they | ||
| arrive rather than at serialization, so a wire-supplied header is never retained | ||
| in full. No length limit is imposed: the 512 characters vendors should be able | ||
| to propagate is a floor on capability rather than a ceiling on output, so | ||
| truncating to fit a transport limit stays the caller's decision. |
Member
There was a problem hiding this comment.
Same here - I think it's focusing more on how it's implemented, rather than what people should use it for and how.
| /// The maximum number of members in a W3C <c>tracestate</c> value, and so the most this type | ||
| /// ever retains. | ||
| /// </summary> | ||
| private const int MemberLimit = 32; |
Member
There was a problem hiding this comment.
MaxMembers or something like that?
Comment on lines
+40
to
+45
| private const int KeyLengthLimit = 256; | ||
|
|
||
| /// <summary> | ||
| /// The maximum length of a value, per the <c>value = 0*255(chr) nblk-chr</c> production. | ||
| /// </summary> | ||
| private const int ValueLengthLimit = 256; |
Member
There was a problem hiding this comment.
Similarly, Max{X}Length for consistency with other code in the codebase and .NET that use Max and Min prefixes (like int.MaxValue).
Comment on lines
+49
to
+52
| // Never longer than MemberLimit: Parse and Set both stop filling it there, and Remove only ever | ||
| // shrinks it. Bounding it here rather than at serialization keeps a wire-supplied header from | ||
| // being retained in full. It is sized exactly at every allocation, because nothing is ever | ||
| // appended to an instance once it exists. |
Comment on lines
+316
to
+317
| var member = (comma < 0 ? remaining : remaining.Slice(0, comma)).Trim(); | ||
| remaining = comma < 0 ? default : remaining.Slice(comma + 1); |
Member
There was a problem hiding this comment.
I think this would be more readable as an if (command >= 0) block containing the two mutations.
var member = remaining;
if (comma >=0 )
{
member = member.Slice(0, comma);
remaining = remaining.Slice(comma + 1);
}
member = member.Trim();|
|
||
| // Counts the members a header yields, stopping at MemberLimit so that the right-most ones are | ||
| // never taken on. Empty members are dropped rather than counted. | ||
| private static int CountMembers(ReadOnlySpan<char> tracestate) |
Member
There was a problem hiding this comment.
This could be a local method to the one above as it's only used in one place.
Comment on lines
+346
to
+348
| var comma = remaining.IndexOf(','); | ||
| var member = (comma < 0 ? remaining : remaining.Slice(0, comma)).Trim(); | ||
| remaining = comma < 0 ? default : remaining.Slice(comma + 1); |
Member
There was a problem hiding this comment.
Similar comment to above.
| /// One member of a <c>tracestate</c> list: either a key and its value, or a run of text kept | ||
| /// verbatim because it did not match the grammar. | ||
| /// </summary> | ||
| private readonly struct Member |
Member
There was a problem hiding this comment.
Simplify with primary constructor?
Comment on lines
+10
to
+18
| // "There can be a maximum of 32 list-members in a list." | ||
| // https://www.w3.org/TR/2021/REC-trace-context-1-20211123/#tracestate-header | ||
| private const int MemberLimit = 32; | ||
|
|
||
| // key = ( lcalpha / DIGIT ) 0*255 ( keychar ), so 256 characters is the maximum. | ||
| private const int KeyLengthLimit = 256; | ||
|
|
||
| // value = 0*255(chr) nblk-chr, so 256 characters is the maximum. | ||
| private const int ValueLengthLimit = 256; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3091
Changes
Adds
OpenTelemetry.Trace.W3CTraceState. It is a W3C tracestate header exposing the four operations the Trace State spec defines..NET exposes tracestate only as the raw ActivityContext.TraceState string. A custom sampler would have to re-implement the W3C specification itself.
OtelTraceStateis left as-is.Notes:
W3Cprefix was added even though naming itTraceStatewould compile as well. The spec's required get/add/update/delete on TraceState is still unimplemented in .NET, if core adds TraceState toOpenTelemetry.Trace, every consumer of both packages will get CS0104. Happy to change the naming, if needed.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes